test(data): source freshness on all bronze connectors (#1322) - #1346
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAll 21 bronze Airbyte connector Changesdbt Source Freshness Rollout
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Coordinating — I have a ready "runtime/alerting" layer that builds directly on this PR. This PR nails the declaration half of #1322 (per-source
None of that exists in #1322's scope (which is declaration + coverage gate), so it's additive. Two things to align:
One design note for the record: my original took the inherited-default route ( Want me to open the follow-up PR once this merges, or fold the cron into this branch? |
|
Thanks — agreed on all of it, and good that the runtime half is already written. On the two alignment points:
For tracking: the runtime/alerting CronWorkflow is the "scheduled bronze freshness" item in the stabilization plan (the monitoring line, #949), so it slots in there as the operational counterpart to this declaration gate. And confirmed your read on |
|
Heads-up before this lands: flat The check correctly catches "sync fully stopped" for true incremental connectors. But for any connector whose API returns a fixed reporting window that the sync re-fetches every run (M365 Graph reports, ChatGPT Team, Claude Team), Measured just now on the Eternal ClickHouse (
Right now M365 business data is 4.5 days stale on this install, but This matters because the false-green sources are the AI-usage connectors that Insight's value story leans on. The connector config is misleading here too — M365 declares Proposal: adopt the per-source model from the earlier #256 work as the declaration here — it's demonstrably the correct instrument on exactly these connectors. Concretely, additive to this PR's per-source structure:
I have the full per-connector |
|
This is the right catch, and it changes my call from earlier in the thread — thank you for dogfooding it on live data. You're right that So I'm reversing my "keep it flat, declaration-only is fine as-is" position: the per-source + tiered model belongs in this PR, not a follow-up. A freshness gate that reads green while M365 business data is 4.5 days stale is worse than no gate — and it's worst on the AI-usage connectors that matter most. My earlier 36h/72h reasoning only held for the true-incremental connectors ( Please push the #256 per-connector mapping straight onto this branch (
The runtime/alerting CronWorkflow stays the separate follow-up we discussed — this keeps that PR sitting on a declaration that's actually correct. If pushing to the branch is awkward from your side, open it as a PR into |
|
Opened it as a PR into this branch: SharedQA#1 — anchors the windowed connectors (m365 / chatgpt_team / claude_team verified, the rest column-verified or inferred-and-marked) on their business-date column with tiered thresholds; incrementals untouched. The PR body breaks down the three confidence tiers and the one caveat (no end-to-end |
9be8468 to
e4a093a
Compare
…ric#1322) Adds dbt source freshness to all 21 bronze_* connector sources (warn 36h / error 72h, loaded_at_field=_airbyte_extracted_at). A stopped sync currently goes unnoticed — dashboards just render stale numbers with no error. `dbt source freshness` now flags it. Verified _airbyte_extracted_at exists on the ingested bronze tables; freshness is opt-in (its own command), so sources not yet ingested (constructorfabric#1319) don't affect the normal build/test run. First step of the data-test coverage push (EPIC constructorfabric#1321). Next: enforced contracts (data_type) on silver — that needs the warehouse-gated CI dbt run to validate the declarations, so it lands once data-checks.yml points at a populated ClickHouse. Signed-off-by: Kenan Salim <kenan.salim@rolos.com>
…eshness Flat _airbyte_extracted_at is false-green on windowed/vendor-analytics connectors: the API re-emits a fixed reporting window every sync, so the extracted-at timestamp tracks sync liveness, not data freshness. Verified live (Eternal): M365 business data 4.5d stale while extracted_at 10.6h fresh; same on ChatGPT Team / Claude Team. Incrementals (git/jira/youtrack) are fine (ext_age approx biz_age) and keep _airbyte_extracted_at. Per connector: - m365 / chatgpt_team / claude_team: VERIFIED on live data — anchor on reportRefreshDate / date,snapshot_date / metric_date, report tier (72/120h). - slack / cursor / zoom: column verified in CH, behavior inferred — anchor on date / day / event-time; slack report_extended (120/168h), zoom event (96/168h). - openai / claude-admin / claude-enterprise / github-copilot / confluence / outline: INFERRED, not live-verified (no rows in any reachable install; connector config is not a reliable signal, cf. M365 P0D). Anchored on the cursor business column with conservative tiers, marked inline. - rosters/lookups keep _airbyte_extracted_at (full-refresh → sync-liveness signal). Refs constructorfabric#1321, constructorfabric#1322. Builds the correct declaration the runtime gate (constructorfabric#949) sits on. Signed-off-by: Kenan Salim <kenan.salim@rolos.com>
e4a093a to
f9bac1d
Compare
|
One more before this lands — reviewing the merged anchors I caught a latent bug and opened SharedQA#2 against this branch:
openai is empty in the dogfood install so it wouldn't fail today, but it's a latent false-positive. Worth folding #2 in before #1346 merges so the gate ships correct. All other inferred anchors were re-checked against the connectors' silver models and parse fine. |
Review of constructorfabric#1346 caught two anchor issues in the per-source freshness blocks: - openai: `bucket_start_time` is Unix seconds (integer), per the connector's own dbt models (toDate(fromUnixTimestamp(CAST(bucket_start_time AS UInt32)))). parseDateTimeBestEffortOrNull expects a string and would runtime-error on all usage/cost tables. Switch to fromUnixTimestamp(CAST(... AS UInt32)). - confluence/outline wiki_pages: align to parseDateTime64BestEffortOrNull(updated_at, 3) to match each connector's own model convention (functionally equivalent for freshness, but consistent). Follow-up to PR constructorfabric#1 on this branch; keeps constructorfabric#1346 correct before it merges to main.
Review of constructorfabric#1346 caught two anchor issues in the per-source freshness blocks: - openai: `bucket_start_time` is Unix seconds (integer), per the connector's own dbt models (toDate(fromUnixTimestamp(CAST(bucket_start_time AS UInt32)))). parseDateTimeBestEffortOrNull expects a string and would runtime-error on all usage/cost tables. Switch to fromUnixTimestamp(CAST(... AS UInt32)). - confluence/outline wiki_pages: align to parseDateTime64BestEffortOrNull(updated_at, 3) to match each connector's own model convention (functionally equivalent for freshness, but consistent). Follow-up to PR #1 on this branch; keeps constructorfabric#1346 correct before it merges to main. (cherry picked from commit cb13e9c) Signed-off-by: Kenan Salim <kenan.salim@rolos.com>
…trap-detector, driver notifications Runtime/alerting half of bronze source freshness (EPIC constructorfabric#1321, closes constructorfabric#949). Sits on top of the per-source freshness declarations from constructorfabric#1346: - dbt-source-freshness-check CronWorkflow (daily, parses sources.json) - driver-based notifications (webhook/zulip/slack/teams/email) - freshness-trap-detect.py runtime trap detector - ingestion-monitoring domain PRD/DESIGN/FEATURE + operator runbook Thresholds owned by the per-source declarations, not this PR. Signed-off-by: Alexey Panfilov <alexey.panfilov@constructor.tech>
The PRD/DESIGN/FEATURE/MONITORING docs described the original cohesive design (inherited +freshness in dbt_project.yml, env_var-driven thresholds, lint-bronze-freshness.py CI). That model was dropped in favor of the split agreed with QA: - declarations + literal per-source thresholds owned by the connector schemas (EPIC constructorfabric#1321/constructorfabric#1322 — PR constructorfabric#1346 + PR SharedQA#1), coverage counted by the QA dbt_coverage.py gate - this PR owns runtime/alerting only (CronWorkflow, parser, trap detector, driver notifications) Also folds in the empirically-verified false-green finding (windowed vendor-analytics connectors must anchor on a business-date column, not _airbyte_extracted_at) and corrects cyberfabric -> constructorfabric refs. Signed-off-by: Alexey Panfilov <alexey.panfilov@constructor.tech>
What
Adds dbt source freshness to all 21
bronze_*connector sources —warn_after: 36h,error_after: 72h,loaded_at_field: _airbyte_extracted_at.Why
A stopped/slow sync is currently invisible: the dashboard just shows stale numbers with no error.
dbt source freshnessturns that into a signal. Implements the bronze half of #1322 (EPIC #1321).Notes
dbt source freshness), not part ofdbt build/test— so sources not yet ingested (data: wired-but-empty connectors (Slack, Zoom, Zulip-Proxy) break silver build and yield no metrics — seed or deregister #1319) don't break the normal run._airbyte_extracted_atexists on the ingested bronze tables.data_type) on silver models — those need the warehouse-gated CI dbt run to validate the type declarations, so they land oncedata-checks.ymlruns against a populated ClickHouse (can't be validated on a machine without dbt).Draft until the freshness thresholds are confirmed with the team.
Summary by CodeRabbit